Size the host memory-baseline hard gate to each module's observed swing - #5598
Conversation
The per-module post-GC boundary delta is dominated by non-deterministic settle-GC drain timing, so heavy modules swing run-to-run by >100MB with no leak. When a module's entire recent window is negative, its mean and ceiling both floor to zero and the hard gate collapses to "any single reading over +50MB fails" — so a module that routinely swings from -120MB to -8MB gets blocked the one run it happens to land at +54MB. Fold the recent window's peak-to-peak spread into the hard threshold: a run must clear the ceiling by more than the module has already shown it can move on its own before it blocks. Tight-window modules (spread below the absolute threshold) are unaffected; a genuine step past the observed swing still fails. The soft, mean-anchored warning is untouched, so upward trends still surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…math The file header and the failures-table heading still described the gate as a flat ">2x baseline or +50MB" rule, which stopped being accurate once the hard gate moved to a ceiling-relative measurement. State the actual condition — a delta clears the recent ceiling by more than the larger of +50MB, 100% of the baseline, or the module's own observed swing — and correct the inline note that implied the absolute term blocks any positive reading rather than one over 50MB. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
burieberry
left a comment
burieberry
left a comment
There was a problem hiding this comment.
Code review
The new baselineSpread mechanism does exactly what the PR intends and is well-documented — no bugs here. Two design questions on whether the hard gate ends up too permissive in edge cases. Both are about the build-blocking gate only; I verified the soft/warning gate (anchored to the mean at +5MB) still fires in every scenario below, so these regressions still surface in CI — they just stop blocking it.
1. A spread derived entirely from negative samples is granted as positive headroom
baselineCeiling floors negative windows to 0 (negatives treated as noise), but baselineSpread is not floored — so a window that lived entirely below zero still contributes its full width as positive growth room.
Worked example with the window from your own comment, [-73, -74, -8, -19, -120]:
effectiveBase → 0,ceiling → 0, butspread = -8 − (-120) = 112hardThreshold = max(50, 0, 112) = 112- ⇒ the module can jump from ~0 to +111MB and still clear the hard gate, despite never once exhibiting a positive delta in its window.
Two things fold together here: the spread's magnitude is set by a single deep-negative sample (min drives it), and that negative-range width is applied as positive headroom above a zero-floored ceiling. Is treating negatives as noise for the ceiling but counting them at full width for the spread the intended asymmetry? If a window is all-negative, would flooring the spread (or sizing it only from the non-negative samples) better match the "negatives are noise" reasoning already applied to the ceiling?
2. A monotonic upward trend inflates the very gate meant to catch it
spread can't distinguish run-to-run noise from a steady climb. For a module leaking on every run, e.g. [10, 30, 50, 70, 90]:
ceiling = 90,spread = 80,hardThreshold = max(50, 50, 80) = 80- ⇒ the next run must reach
ceiling + threshold = 170MBbefore it hard-fails.
The trend that signals the leak widens the threshold that's supposed to catch it. The soft gate still warns (so it's visible), but the build-blocking gate keeps relaxing as the leak grows. Worth considering whether a trend/slope check should bound the spread term, or whether spread should be computed from a detrended window.
Neither is blocking — the gate still fails on truly unprecedented jumps, and warnings still surface trends. Mostly want to confirm the negative-window headroom in (1) is deliberate, since that's the case where the gate loosens most.
🤖 Review generated with Claude Code
confirm it is deliberate |
2 participants
The "Host Memory Baseline" check compares each test module's post-GC heap boundary delta against a rolling window of recent main runs and hard-fails on a large regression. That metric is dominated by non-deterministic settle-GC drain timing at the module boundary, so heavy modules routinely swing run-to-run by well over 100MB without leaking anything — a negative reading just means the previous module's garbage got reclaimed inside this module's window.
The hard gate already anchors to the recent ceiling (max sample) rather than the mean, precisely so a high-variance module can't be blocked by a value it has already exhibited. But when a module's entire recent window is negative, both its mean baseline and its ceiling floor to zero, and the gate collapses to "any single reading over +50MB fails." A module that has been sitting at samples like
[-73, -74, -8, -19, -120]then gets blocked the one run it lands at+54:The baseline showing
0.0and the change being the raw measured value is the tell: nothing about the module's actual behaviour is informing the threshold. That window spans 112MB peak-to-peak, so a one-off +54 is squarely inside the module's own noise.This folds the recent window's peak-to-peak spread into the hard threshold, alongside the existing absolute and relative terms. A run now has to clear the ceiling by more than the module has already shown it can move on its own before it blocks. Behaviour at the edges:
This is a strict relaxation of the current gate. The measured delta and the ceiling it's compared against are unchanged, and the threshold only ever grows (the added spread term is non-negative), so
hardThresholdcan only increase. No module that clears the hard gate today can begin to fail under this change — it lands with no re-baselining and introduces no new reds.That monotonicity is also why the spread is layered on top of the ceiling anchor rather than replacing it with a single mean anchor. Anchoring the excursion at the mean would tighten the wide-swing modules, but it would begin blocking moderate-variance modules that pass today — e.g. a
[40, 60, 40, 60, 50]window would hard-fail at+100MB, where both the current gate and this change pass. The cost of keeping the ceiling anchor is that a very wide window tolerates a correspondingly large excursion before it hard-blocks; that excursion still prints as a non-blocking warning, and a sustained regression re-crosses the gate once it folds into the rolling baseline.Verified against reconstructed data from the failing run and synthetic leak/low-variance cases: the false failure drops to a non-blocking warning, while both a low-variance step-change and a noisy value that clears its own swing still hard-fail.